Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Oauth token refresh & Quarkus Log exception #75

Merged
merged 6 commits into from
May 15, 2024
Merged

Fix Oauth token refresh & Quarkus Log exception #75

merged 6 commits into from
May 15, 2024

Conversation

SravanThotakura05
Copy link
Collaborator

@ozangunalp - To validate the OAuth token refresh issue, i have left the extension to run since morning and i haven't seen the issue of disconnection. Integration test is not added as we were seeing this issue after few hours.

My observation during testing.

  1. Buffer overflow - Ideally this will not occur in real use as user need to set the interval so that only one request is made before token expiry. If they are setting smaller intervals then buffer might overflow due to large number of requests.
  2. Solace Reconnection - Reconnection might happen on idle connections depending on keep alive configuration. To avoid unauthorised token we are updating the token based on Solace Reconnect Listener interface.
  3. Removed the filter cause in Token refresh method and updated the call method as it already has if condition check.

On Quarkus Log exception since we were not able to reproduce I have added empty beans.xml as recommended by you

@ozangunalp
Copy link
Collaborator

All this looks good.

For the overflow issue, I think this can happen when the token refresh takes too much time. Normally, it should not surpass the HTTP connection timeout. As a safeguard, we may add a onOverflow().drop() to the tick and add a timeout after token refresh calls .ifNoItem().after(some duration).fail()

For the log issue, let's see if the beans.xml resolves the problem, otherwise, we can fall back to using the normal logger and report the issue to the Quarkus repo.

@SravanThotakura05
Copy link
Collaborator Author

@ozangunalp - Added new property to configure refresh timeout for the requests and fail on no response.

Commit - 0995bc0

@SravanThotakura05 SravanThotakura05 merged commit ff57ebe into main May 15, 2024
3 checks passed
@SravanThotakura05 SravanThotakura05 deleted the fix-oauth-token-refresh branch July 23, 2024 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ozangunalp ozangunalp ozangunalp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SravanThotakura05 @ozangunalp